Skip to content

[TRTLLM-8638][fix] Add failed cases into waives.txt #8669

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Oct 27, 2025
Merged

[TRTLLM-8638][fix] Add failed cases into waives.txt #8669

merged 2 commits into from
Oct 27, 2025
+16 −3

Conversation

@xinhe-nv
Copy link
Collaborator

@xinhe-nv xinhe-nv commented Oct 26, 2025 •
edited by coderabbitai bot
Loading

waive failed cases.

Summary by CodeRabbit

  • Tests
    • Removed one test entry from the active test suite
    • Expanded the test skip list with multiple entries for specific test configurations and hardware platforms

@xinhe-nv xinhe-nv force-pushed the user/qa/post_update_waive_20251026_LLM_FUNCTION_TEST_1566 branch 2 times, most recently from 4cc356e to 2cb60f1 Compare October 27, 2025 02:07
@xinhe-nv xinhe-nv requested a review from jieli-matrix October 27, 2025 02:08
@xinhe-nv xinhe-nv marked this pull request as ready for review October 27, 2025 02:08
@xinhe-nv
Copy link
Collaborator Author

/bot run --skip-test

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 27, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

This PR modifies test configuration files to manage test execution: removing a single test entry from the LLM function test list and adding SKIP entries for specific failing tests in the waives list.

Changes

Cohort / File(s) Summary
LLM Function Test List
tests/integration/test_lists/qa/llm_function_nim.txt		 Removed test entry accuracy/test_cli_flow.py::TestMistral7B::test_beam_search
Test Waivers
tests/integration/test_lists/waives.txt		 Added multiple SKIP entries for Gemma3_1BInstruct and GPTOSS tests under full:RTX paths, each with corresponding nvbug references

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Test list modifications are straightforward deletions and additions of test entries with no logic changes or functional modifications
  • Changes are purely administrative in nature (test skipping and removal)

Possibly related PRs

Suggested reviewers

  • crazydemo
  • jieli-matrix
  • LarryXFly

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description is largely incomplete and does not adequately follow the required template. The provided description consists of only "waive failed cases." which is insufficient. The description is missing the required Description section (which should explain the issue and solution in detail), the Test Coverage section (which should list relevant tests being waived), and the PR Checklist. While the title is descriptive, the description itself lacks the necessary detail and structure to justify the changes made. Please expand the PR description to include: (1) a detailed Description section explaining which test cases failed, why they are being waived, and any relevant context about the failures; (2) a Test Coverage section listing the specific tests being waived (such as the Gemma3_1BInstruct and GPTOSS tests mentioned in the file changes); and (3) completion of the PR Checklist items. This will provide reviewers with the necessary context to understand and validate the changes.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "[TRTLLM-8638][fix] Add failed cases into waives.txt" directly and clearly describes the primary change in the changeset. The raw summary confirms that multiple SKIP entries for failed test cases (Gemma3_1BInstruct and GPTOSS tests) are being added to waives.txt, which aligns with the title's message. The title is specific, concise, and communicates the core intent without unnecessary verbosity. While the changeset includes a secondary removal from llm_function_nim.txt, the title appropriately focuses on the main objective—expanding the waive list—which is consistent with the PR's stated purpose to "waive failed cases."
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tensorrt-cicd
Copy link
Collaborator

PR_Github #22553 [ run ] triggered by Bot. Commit: 2cb60f1

@xinhe-nv xinhe-nv enabled auto-merge (squash) October 27, 2025 02:14
@xinhe-nv xinhe-nv force-pushed the user/qa/post_update_waive_20251026_LLM_FUNCTION_TEST_1566 branch from 2cb60f1 to f812f0a Compare October 27, 2025 02:32
@xinhe-nv xinhe-nv changed the title [None][chore] Add failed cases into waives.txt [TRTLLM-8638][fix] Add failed cases into waives.txt Oct 27, 2025
@xinhe-nv
update waive list
348dc0f 
Signed-off-by: xinhe-nv <[email protected]>
@xinhe-nv
update waives
5e6cbf2 
Signed-off-by: Xin He (SW-GPU) <[email protected]>
@xinhe-nv xinhe-nv force-pushed the user/qa/post_update_waive_20251026_LLM_FUNCTION_TEST_1566 branch from f812f0a to 5e6cbf2 Compare October 27, 2025 03:43
@tensorrt-cicd
Copy link
Collaborator

PR_Github #22553 [ run ] completed with state SUCCESS. Commit: 2cb60f1
/LLM/main/L0_MergeRequest_PR pipeline #17001 (Partly Tested) completed with status: 'SUCCESS'

@xinhe-nv
Copy link
Collaborator Author

/bot reuse-pipeline

@tensorrt-cicd
Copy link
Collaborator

PR_Github #22585 [ reuse-pipeline ] triggered by Bot. Commit: 5e6cbf2

@tensorrt-cicd
Copy link
Collaborator

PR_Github #22585 [ reuse-pipeline ] completed with state SUCCESS. Commit: 5e6cbf2
Reusing PR_Github #22553 (Partly Tested) for commit 5e6cbf2

@xinhe-nv xinhe-nv merged commit 0ac5cbc into NVIDIA:main Oct 27, 2025
5 checks passed
@xinhe-nv xinhe-nv deleted the user/qa/post_update_waive_20251026_LLM_FUNCTION_TEST_1566 branch October 27, 2025 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jieli-matrix jieli-matrix jieli-matrix approved these changes

@crazydemo crazydemo Awaiting requested review from crazydemo

@LarryXFly LarryXFly Awaiting requested review from LarryXFly

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@xinhe-nv @tensorrt-cicd @jieli-matrix